-
Notifications
You must be signed in to change notification settings - Fork 0
feat: Console Authentication System — @object-ui/auth package, route guards, system admin pages #421
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
…, AuthGuard, forms, and token injection Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
…Guard, dynamic user context, and system admin pages Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
…rove error handling in createAuthClient Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
…nt to comply with rules of hooks Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
📦 Bundle Size Report
Size Limits
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Implements the Console “login-to-usage” flow by introducing a new @object-ui/auth workspace package (provider/hook/guard + basic auth forms), wiring the Console app to use auth context + guarded routes, and scaffolding initial system administration pages under /apps/:appName/system/*.
Changes:
- Added
@object-ui/authpackage:AuthProvider/useAuth,AuthGuard, REST client helpers, and basic login/register/forgot-password UI components. - Integrated auth into
apps/consolerouting and sidebar user display/sign-out. - Added initial “System” pages and object definitions scaffolding for admin/profile/audit areas.
Reviewed changes
Copilot reviewed 27 out of 28 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| pnpm-lock.yaml | Adds workspace link for @object-ui/auth and updates lock snapshots accordingly. |
| packages/auth/package.json | Defines new @object-ui/auth package build/test/lint metadata and exports. |
| packages/auth/tsconfig.json | Adds package-level TS build config consistent with other workspace packages. |
| packages/auth/src/index.ts | Public API exports for the new auth package. |
| packages/auth/src/types.ts | Defines auth domain types + getUserInitials helper. |
| packages/auth/src/AuthContext.ts | Declares the auth React context contract. |
| packages/auth/src/AuthProvider.tsx | Implements auth state management + context provider wiring. |
| packages/auth/src/useAuth.ts | Hook to consume the auth context. |
| packages/auth/src/AuthGuard.tsx | Route/content guard component based on auth/role state. |
| packages/auth/src/createAuthClient.ts | REST client factory for auth endpoints. |
| packages/auth/src/createAuthenticatedFetch.ts | Fetch wrapper to inject session bearer token. |
| packages/auth/src/LoginForm.tsx | Tailwind-styled login form consuming useAuth(). |
| packages/auth/src/RegisterForm.tsx | Tailwind-styled registration form consuming useAuth(). |
| packages/auth/src/ForgotPasswordForm.tsx | Tailwind-styled forgot password flow consuming useAuth(). |
| packages/auth/src/UserMenu.tsx | Headless-ish user info/actions block consuming useAuth(). |
| apps/console/package.json | Adds @object-ui/auth dependency to the Console app. |
| apps/console/vite.config.ts | Adds Vite aliases for @object-ui/auth (and related future packages). |
| apps/console/src/App.tsx | Wraps Console in AuthProvider, adds public auth routes, guards app routes, adds system routes, and feeds auth user into ExpressionProvider. |
| apps/console/src/components/AppSidebar.tsx | Replaces hardcoded user info with useAuth() + sign-out integration. |
| apps/console/src/pages/LoginPage.tsx | Adds login page wrapper around LoginForm. |
| apps/console/src/pages/RegisterPage.tsx | Adds register page wrapper around RegisterForm. |
| apps/console/src/pages/ForgotPasswordPage.tsx | Adds forgot-password page wrapper around ForgotPasswordForm. |
| apps/console/src/pages/system/systemObjects.ts | Adds system object metadata scaffolding (users/orgs/roles/permissions/audit). |
| apps/console/src/pages/system/UserManagementPage.tsx | Adds initial user management scaffold page. |
| apps/console/src/pages/system/OrgManagementPage.tsx | Adds initial org management scaffold page. |
| apps/console/src/pages/system/RoleManagementPage.tsx | Adds initial role management scaffold page. |
| apps/console/src/pages/system/AuditLogPage.tsx | Adds initial audit log scaffold page. |
| apps/console/src/pages/system/ProfilePage.tsx | Adds initial profile edit scaffold page. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
| /** | ||
| * Options for creating an authenticated adapter. | ||
| */ | ||
| export interface AuthenticatedAdapterOptions { | ||
| /** Base URL for the ObjectStack API */ | ||
| baseUrl: string; | ||
| /** Auth client to get session tokens from */ | ||
| authClient: AuthClient; | ||
| /** Additional adapter options */ | ||
| [key: string]: unknown; | ||
| } |
Copilot
AI
Feb 10, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AuthenticatedAdapterOptions is exported but not used by createAuthenticatedFetch (which only accepts authClient). This adds API surface without functionality. Either remove this interface or update createAuthenticatedFetch to accept an options object and use the fields (e.g., to scope auth injection to a base URL).
| <Route path="system/users" element={<UserManagementPage />} /> | ||
| <Route path="system/organizations" element={<OrgManagementPage />} /> | ||
| <Route path="system/roles" element={<RoleManagementPage />} /> | ||
| <Route path="system/audit-log" element={<AuditLogPage />} /> |
Copilot
AI
Feb 10, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
System admin routes are mounted without any admin role guard, so any authenticated user can navigate to /apps/:appName/system/users|organizations|roles. The pages only hide the “Add …” button, which doesn’t meet the “admin-gated” intent and is a security gap. Wrap the admin-only routes (users/orgs/roles, possibly audit-log) in an AuthGuard requiredRoles={["admin"]} (or a permissions-based guard) and provide an explicit access-denied fallback.
| <Route path="system/users" element={<UserManagementPage />} /> | |
| <Route path="system/organizations" element={<OrgManagementPage />} /> | |
| <Route path="system/roles" element={<RoleManagementPage />} /> | |
| <Route path="system/audit-log" element={<AuditLogPage />} /> | |
| <Route | |
| path="system/users" | |
| element={ | |
| <AuthGuard | |
| requiredRoles={["admin"]} | |
| fallback={ | |
| <Empty> | |
| <EmptyTitle>Access denied</EmptyTitle> | |
| </Empty> | |
| } | |
| > | |
| <UserManagementPage /> | |
| </AuthGuard> | |
| } | |
| /> | |
| <Route | |
| path="system/organizations" | |
| element={ | |
| <AuthGuard | |
| requiredRoles={["admin"]} | |
| fallback={ | |
| <Empty> | |
| <EmptyTitle>Access denied</EmptyTitle> | |
| </Empty> | |
| } | |
| > | |
| <OrgManagementPage /> | |
| </AuthGuard> | |
| } | |
| /> | |
| <Route | |
| path="system/roles" | |
| element={ | |
| <AuthGuard | |
| requiredRoles={["admin"]} | |
| fallback={ | |
| <Empty> | |
| <EmptyTitle>Access denied</EmptyTitle> | |
| </Empty> | |
| } | |
| > | |
| <RoleManagementPage /> | |
| </AuthGuard> | |
| } | |
| /> | |
| <Route | |
| path="system/audit-log" | |
| element={ | |
| <AuthGuard | |
| requiredRoles={["admin"]} | |
| fallback={ | |
| <Empty> | |
| <EmptyTitle>Access denied</EmptyTitle> | |
| </Empty> | |
| } | |
| > | |
| <AuditLogPage /> | |
| </AuthGuard> | |
| } | |
| /> |
| export function ProfilePage() { | ||
| const { user, updateUser, isLoading } = useAuth(); | ||
| const [name, setName] = useState(user?.name ?? ''); | ||
| const [saved, setSaved] = useState(false); | ||
| const [error, setError] = useState<string | null>(null); |
Copilot
AI
Feb 10, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
name state is initialized from user?.name only on the first render. Because user is typically loaded asynchronously, the input can stay empty even after user becomes available, and saving would overwrite the profile name with an empty string. Sync the local name state when user changes (e.g., via useEffect) or derive the input value directly from user until the user edits it.
| /** Token expiry timestamp */ | ||
| expiresAt?: Date; |
Copilot
AI
Feb 10, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AuthSession.expiresAt is typed as a Date, but createAuthClient returns raw response.json() data without converting strings/numbers to Date instances. This makes the runtime value inconsistent with the declared type. Consider changing the type to a wire-friendly format (e.g., ISO string / epoch ms) or parsing it into a Date in createAuthClient.
| /** Token expiry timestamp */ | |
| expiresAt?: Date; | |
| /** Token expiry timestamp (ISO 8601 string from the auth server) */ | |
| expiresAt?: string; |
| return async (input: RequestInfo | URL, init?: RequestInit) => { | ||
| const session = await authClient.getSession(); | ||
| const headers = new Headers(init?.headers); | ||
| if (session?.session?.token) { | ||
| headers.set('Authorization', `Bearer ${session.session.token}`); | ||
| } |
Copilot
AI
Feb 10, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
createAuthenticatedFetch calls authClient.getSession() for every outgoing request. With the provided createAuthClient, getSession() performs a network request, which can create significant overhead (and added latency) for chatty adapters. Consider caching the session/token in memory (with invalidation on auth state changes) or accepting a synchronous token getter sourced from AuthProvider state.
| return async (input: RequestInfo | URL, init?: RequestInit) => { | |
| const session = await authClient.getSession(); | |
| const headers = new Headers(init?.headers); | |
| if (session?.session?.token) { | |
| headers.set('Authorization', `Bearer ${session.session.token}`); | |
| } | |
| // Simple in-memory cache to avoid calling getSession() on every request. | |
| // This keeps the API surface the same while reducing network overhead. | |
| const CACHE_TTL_MS = 60_000; // 60 seconds | |
| let cachedSession: Awaited<ReturnType<AuthClient['getSession']>> | null = null; | |
| let lastSessionFetch = 0; | |
| const getCachedSession = async () => { | |
| const now = Date.now(); | |
| if (!cachedSession || now - lastSessionFetch > CACHE_TTL_MS) { | |
| cachedSession = await authClient.getSession(); | |
| lastSessionFetch = now; | |
| } | |
| return cachedSession; | |
| }; | |
| return async (input: RequestInfo | URL, init?: RequestInit) => { | |
| const session = await getCachedSession(); | |
| const headers = new Headers(init?.headers); | |
| if (session?.session?.token) { | |
| headers.set('Authorization', `Bearer ${session.session.token}`); | |
| } |
| async function request<T>(path: string, options?: RequestInit): Promise<T> { | ||
| const url = `${baseURL}${path}`; | ||
| const response = await fetchFn(url, { | ||
| ...options, | ||
| headers: { | ||
| 'Content-Type': 'application/json', | ||
| ...options?.headers, | ||
| }, | ||
| credentials: 'include', | ||
| }); | ||
|
|
||
| if (!response.ok) { | ||
| const body = await response.json().catch(() => null); | ||
| const message = (body && typeof body === 'object' && 'message' in body) | ||
| ? String(body.message) | ||
| : `Auth request failed with status ${response.status}`; | ||
| throw new Error(message); | ||
| } | ||
|
|
||
| return response.json(); | ||
| } | ||
|
|
||
| return { | ||
| async signIn(credentials: SignInCredentials) { | ||
| return request<{ user: AuthUser; session: AuthSession }>('/sign-in/email', { | ||
| method: 'POST', | ||
| body: JSON.stringify(credentials), | ||
| }); | ||
| }, | ||
|
|
||
| async signUp(data: SignUpData) { | ||
| return request<{ user: AuthUser; session: AuthSession }>('/sign-up/email', { | ||
| method: 'POST', | ||
| body: JSON.stringify(data), | ||
| }); | ||
| }, | ||
|
|
||
| async signOut() { | ||
| await request('/sign-out', { method: 'POST' }); | ||
| }, |
Copilot
AI
Feb 10, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
request() always calls response.json() on success. Several client methods (signOut, forgotPassword, resetPassword) conceptually return void, so this will throw if those endpoints respond with 204 No Content or any non-JSON body. Consider handling empty bodies (e.g., check status/Content-Length/Content-Type and skip JSON parsing) or splitting into requestJson vs requestVoid.
| /** | ||
| * Create an auth client instance. | ||
| * | ||
| * This factory creates an abstraction layer over the authentication provider. | ||
| * It is designed to work with better-auth but can be adapted to any auth backend | ||
| * that exposes standard REST endpoints for sign-in, sign-up, sign-out, and session management. | ||
| * | ||
| * @example | ||
| * ```ts | ||
| * const authClient = createAuthClient({ baseURL: '/api/auth' }); | ||
| * const { user, session } = await authClient.signIn({ email, password }); | ||
| * ``` | ||
| */ | ||
| export function createAuthClient(config: AuthClientConfig): AuthClient { | ||
| const { baseURL, fetchFn = fetch } = config; | ||
|
|
||
| async function request<T>(path: string, options?: RequestInit): Promise<T> { | ||
| const url = `${baseURL}${path}`; | ||
| const response = await fetchFn(url, { | ||
| ...options, | ||
| headers: { | ||
| 'Content-Type': 'application/json', | ||
| ...options?.headers, | ||
| }, | ||
| credentials: 'include', | ||
| }); | ||
|
|
||
| if (!response.ok) { | ||
| const body = await response.json().catch(() => null); | ||
| const message = (body && typeof body === 'object' && 'message' in body) | ||
| ? String(body.message) | ||
| : `Auth request failed with status ${response.status}`; | ||
| throw new Error(message); | ||
| } | ||
|
|
||
| return response.json(); | ||
| } |
Copilot
AI
Feb 10, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This package introduces critical auth flows (REST client, provider state machine, guard logic) but currently has no unit tests. Since the repo uses Vitest broadly, consider adding tests covering: createAuthClient error/message extraction, handling of empty/non-JSON responses, AuthProvider session bootstrapping, and AuthGuard role checks.
| export interface AuthGuardProps { | ||
| /** Content to render when user is not authenticated */ | ||
| fallback?: React.ReactNode; | ||
| /** Required roles (user must have at least one) */ | ||
| requiredRoles?: string[]; | ||
| /** Required permissions (user must have all) */ | ||
| requiredPermissions?: string[]; | ||
| /** Content to render when loading */ | ||
| loadingFallback?: React.ReactNode; | ||
| /** Children to render when authenticated */ | ||
| children: React.ReactNode; | ||
| } |
Copilot
AI
Feb 10, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
requiredPermissions is declared in the props but never enforced. This is misleading for consumers and can result in routes/components being treated as protected when they aren’t. Either implement permission checks (e.g., by integrating with the permissions system or accepting a hasPermission callback) or remove the prop from AuthGuardProps.
| /** Path to redirect to when not authenticated */ | ||
| redirectTo?: string; |
Copilot
AI
Feb 10, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
redirectTo is part of AuthProviderConfig, but AuthProvider doesn’t read or apply it anywhere. If this is meant to be the canonical redirect target for unauthenticated users, wire it into AuthGuard usage (or expose it via context); otherwise remove it to avoid a “dead” config option.
| /** Path to redirect to when not authenticated */ | |
| redirectTo?: string; |
Implements roadmap item 1.5: complete login-to-usage flow per
CONSOLE_AUTH_PLAN.md. Replaces hardcoded{ name: 'John Doe', role: 'admin' }with real auth context, adds route protection, and scaffolds system administration UI.@object-ui/authpackage (new)/sign-in/email,/sign-up/email,/get-session, etc.)Bearertoken from session intoObjectStackAdapterConsole integration
AuthProvider, added/login,/register,/forgot-passwordpublic routes/apps/:appName/*withAuthGuard fallback={<Navigate to="/login" />}AppContentreadsuseAuth()for dynamicExpressionProvideruser contextAppSidebarfooter now renders real user data + functional sign-out viauseAuth()@object-ui/auth,@object-ui/permissions,@object-ui/tenantSystem administration pages
systemObjects.ts— Schema definitions forsys_user,sys_org,sys_role,sys_permission,sys_audit_log/apps/:appName/system/*💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.